Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cherry pick chainlink PRs #1549

Merged
merged 9 commits into from
Nov 28, 2024
Merged

Cherry pick chainlink PRs #1549

merged 9 commits into from
Nov 28, 2024

Conversation

RensR
Copy link
Collaborator

@RensR RensR commented Nov 28, 2024

This brings three changes into the ccip repo

  • OZ AccessControl support for RegistryModule
  • MockRouter ExtraArgsV2 support
  • Adds Ownable2Step, a more efficient ownable contract
  • Bump foundry to be in line with chainlink

andrejrakic and others added 3 commits November 28, 2024 12:07
* feat: refactor MockCCIPRouter to support EVMExtraArgsV2

* chore: commit changeset

* CCIP-4288 add changeset
* adds OZ AccessControl support to the registry module

* [Bot] Update changeset file with jira issues

* fix snap

* update version

---------

Co-Authored-By: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
@RensR RensR requested review from makramkd, elatoskinas, RayXpub and a team as code owners November 28, 2024 14:34
Copy link
Contributor

github-actions bot commented Nov 28, 2024

Static analysis results are available

Hey @RensR, you can view Slither reports in the job summary here or download them as artifact here.

Please check them before merging and make sure you have addressed all issues.

}

s_owner = newOwner;
if (pendingOwner != address(0)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: is this path tested? I don't see a test for it


s_ownable2Step.transferOwnership(STRANGER);

assertTrue(STRANGER != s_ownable2Step.owner());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we should check that pendingOwner == STRANGER

assertTrue(STRANGER != s_ownable2Step.owner());

vm.startPrank(STRANGER);
s_ownable2Step.acceptOwnership();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: missing checks:

  • New owner updated
  • Pending owner reset
  • Event

@RensR RensR merged commit 3bf1170 into ccip-develop Nov 28, 2024
164 of 167 checks passed
@RensR RensR deleted the cherry-pick-contracts branch November 28, 2024 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants